I seem to be remembering a somewhat recent major version change in Node.js that process.exit might cause console.log messages to be swallowed because of some async operation. I think this note in https://nodejs.org/api/process.html#process_a_note_on_process_i_o refers to what I mean:

Synchronous writes avoid problems such as output written with console.log() or console.error() being unexpectedly interleaved, or not written at all if process.exit() is called before an asynchronous write completes. See process.exit() for more information.

and from https://nodejs.org/api/process.html#process_process_exit_code

It is important to note that calling process.exit() will force the process to exit as quickly as possible even if there are still asynchronous operations pending that have not yet completed fully, including I/O operations to process.stdout and process.stderr

Now the question here is if log.warn() is sync or async.

If they are, I think the solution is to register with the beforeExit event on process and do the desired output there: https://nodejs.org/api/process.html#process_event_beforeexit

And a matter of style, if exiting because of an error condition, a return code other than 0 (zero) should be used. 1 would be an option.

it’s not an error, the install already happened. It’s only the automated setup which fails and we log instructions on how to manually do it instead, so I thought warning fit better?

log.warn() is synchronous, we don’t write any file, I think this should be fine

we don’t write any file

stdout is a file :)

log.warn() is synchronous

What does it use under the hood?

looks like they use process.stderr.write (https://github.com/npm/npmlog/blob/master/log.js#L12, https://github.com/npm/npmlog/blob/master/log.js#L266)

I didn’t know that console.logs or process.(stderr|stdout).writes could potentially not finish if we call process.exit(), I think I do that quite a bit. I’d love to learn what the proper way to handle this is. I’m not sure if a beforeexit event handler will help because the documentation says

The 'beforeExit' event is not emitted for conditions causing explicit termination, such as calling process.exit()

I misread the beforeExit, sorry!

https://www.nczonline.net/blog/2014/02/04/maintainable-node-js-javascript-avoid-process-exit/ explains the issue.

nodejs/node-v0.x-archive#8329 (comment) Suggests wrapping the process.exit() call in a process.nextTick()

okay I read trough nodejs/node#6456 this is really complicated, now I understand npm’s frustration around Node@6’s release.

I pushed another commit to workaround using process.exit() altogether:
182a818